-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(uiView) autoscroll attribute #715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds an `autoscroll="expr"` attribute to `uiView` just like `ngInclude` has, with the difference that if the `autoscroll` attribute isn't defined then the scroll will happen in order to not break the current behavior.
@ysbaddaden can you please review @dlukez PR (#714) and work together with him to ensure we have one valid option? Seems like your tests are a bit more elegant but @dlukez had a couple that test actual expressions, not sure if that's needed though. |
Yeah this test setup is way better! |
Also, I think this PR allows for dynamically changing the attribute, whereas in mine the autoscroll expression is cached when linking occurs (and $eval'd later). |
I am ft Thanks ----- Reply message ----- — |
Changes to follow ngInclude's behavior more closely (like #714): - memorizes the autoscroll expression, since it's not supposed to be changed once the template is compiled, only the evaluated expression should change; - the page won't autoscroll when the attribute isn't present, which is a breaking change from the current implementation.
Stupid me, I didn't check the current PR and duplicated work. I reviewed #714 and made some changes. I'm caching the expression too, since it's not supposed to be changed once the template is compiled (only the expression should change). |
sry about the spam email. My phone messes up sometimes and does crazy Thanks On Tue, Dec 24, 2013 at 9:13 AM, Julien Portalier
Josh Kurz |
Hey guys, first of all, thanks for your efforts here. However, after further consideration, I don't think the Rather than simply make Therefore I see two options: (1) we can forego this PR in favor of a new implementation, or (2) we can merge this now with the understanding that the implementation will change in a BC-breaking way. Thoughts? |
Yeah, when looking at this I didn't feel like $anchorScroll really seemed to fit either. Something was off. I think the first of those options is the way to go. |
Could it be as simple replacing $anchorScroll with element[0].scrollIntoView()? Might need a $timeout (any other way?) to wait until the browser's rendered so it knows how much of the element to show: http://plnkr.co/edit/21iV1j?p=info -- edit -- |
I created this pull request because I didn't have any control over the scroll for a given uiView: sometimes we don't want any scroll at all (eg: scrolling into view a position:fixed element leads to an unpredictable behavior). So, I'd like the Now I'm fine with using |
Replaces `$anchorScroll` call in `uiView` with a custom provider which scrolls the element attached to `uiView` into the current view instead. This should allow a finer default for designer by scrolling automatically to the activate `uiView` without manipulating anchors. Calling `$uiViewScrollProvider.useAnchorScroll()` will restore the current behavior or calling `$anchorScroll` instead.
@ysbaddaden Awesome! This looks perfect. Thanks a lot for the extra work. |
feat(uiView) autoscroll attribute
Hello everyone, It's great to know that we can now control the scrolling on However I have the following two questions: Is there a way know to which view the viewport will scroll when entering a state with multiple named states or nested states? Secondly, if I want to do the scrolling all by myself (i.e. do |
@MrOrz Since
Currently the default behavior is maintained, such that |
The actual scroll is now a provider and can be configured to use either the old or new behavior; it may also be overriden using a decorator: $provide.decorator('$uiViewScroll', function ($delegate) {
return function (uiViewElement) {
console.log("Manually scroll to:", uiViewElement);
};
}); |
Would we ever want to be able to override a I can think of cases where I'd want to control a view's autoscrolling behavior, depending upon the way that the view was activated. |
longer supported. See angular-ui/ui-router#715 for autoscroll attribute if we need to enable/disable ui-view autoscrolling in certain locations (cc @paglias)
longer supported. See angular-ui/ui-router#715 for autoscroll attribute if we need to enable/disable ui-view autoscrolling in certain locations (cc @paglias)
Im a bit confused by all of this... When I use What is the correct way to have a page load into the viewport starting at the top of the content? |
@blowsie did you solve this issue? I have the same question. |
I ended up implementing my own fix, There has been a few updates since i reported the issue, I haven't had time to see if the issue still occurs in the ui-router code |
Adds an
autoscroll="expr"
attribute touiView
just likengInclude
has, with the difference that it will always autoscroll unlessexpr
is falsy.This should fix #110